Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updates for ipyvolume viewer issues #456

Merged
merged 8 commits into from
Aug 6, 2024

Conversation

Carifio24
Copy link
Member

While Azmé and I were investigating the aspect ratio issue mentioned in #451, we noticed that currently, the displayed volume rendering in the ipyvolume viewer seems to be wrong (in particular, it doesn't match what's displayed in the vispy volume viewer). I believe this might also be a partial cause of #450.

I think this is an issue with axes. As an example, I looked at the reduced TAN C14 cube used in the astropy FITS cube tutorial here. If I open this in vispy, I get
Screenshot_2024-07-01_at_1 38 37_AM_optimized

but with the current ipyvolume viewer I get

Screenshot_2024-07-01_at_1 41 10_AM_optimized

which is what led me to believe that the data was being passed into the ipyvolume widget incorrectly. After rearranging the axes as is done in this PR (note that np.transpose returns a view if possible, so we shouldn't need to worry about data getting copied), we get
Screenshot_2024-07-01_at_1 39 19_AM_optimized

This also fixes #451 for me, in that using the native aspect ratio now behaves the same as it does in vispy.

It's hard to tell how the changes to the ipyvolume viewer code affect the 3D scatter viewer since as #449 points out, it isn't working right now. Only thing I can check is that with these changes, the x/y/z labels indicate that it has a right-handed z-up coordinate system, so that's good.

Copy link

codecov bot commented Jul 2, 2024

Codecov Report

Attention: Patch coverage is 96.00000% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.05%. Comparing base (488287d) to head (d479fe9).
Report is 33 commits behind head on main.

Files with missing lines Patch % Lines
glue_jupyter/common/state3d.py 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #456      +/-   ##
==========================================
+ Coverage   86.04%   86.05%   +0.01%     
==========================================
  Files          90       90              
  Lines        5223     5229       +6     
==========================================
+ Hits         4494     4500       +6     
  Misses        729      729              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Carifio24 Carifio24 requested a review from astrofrog July 11, 2024 14:48
@maartenbreddels
Copy link
Collaborator

It's hard to tell how the changes to the ipyvolume viewer code affect the 3D scatter viewer since as #449 points out, it isn't working right now.

Indeed, that was my worry as well.

@Carifio24 Carifio24 changed the title Update axes of 3D volume rendering display Update axes of 3D viewers Jul 25, 2024
@Carifio24
Copy link
Member Author

Turns out that the scatter viewer does work, albeit with some issues with particular glue components - see #449 (comment). After making similar changes to the scatter viewer as well, the display is now looking the same for me using either the ipyvolume scatter or the vispy scatter viewers.
3d-scatter-ipv
scatter-3d-vispy

@Carifio24
Copy link
Member Author

@maartenbreddels and I had a brief discussion about the scatter issues at today's glue meeting and decided that the best thing to do is to make sure that we pass floats to the ipyvolume scatter. This PR was already manipulating the scatter widget's x/y/z values, so I decided to just add that fix here as well.

It's also now easy to confirm that the axis changes here lead to a right-handed coordinate system (which is what we want).

@Carifio24 Carifio24 linked an issue Aug 1, 2024 that may be closed by this pull request
@Carifio24 Carifio24 changed the title Update axes of 3D viewers Updates for ipyvolume viewer issues Aug 1, 2024
Copy link
Member

@astrofrog astrofrog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for all the work on this!

@astrofrog astrofrog merged commit ba37e39 into glue-viz:main Aug 6, 2024
26 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing viewer options in 3D viewers 3D scatter viewer broken
3 participants